-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement structure preserving dynamical decoupling (dd-v2) #7609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks, @babacry!
@NoureldinYosri would you be able to take a look when you get a chance? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first review ... please address the comments and make the CIs pass
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7609 +/- ##
==========================================
- Coverage 99.38% 99.38% -0.01%
==========================================
Files 1091 1091
Lines 97815 98288 +473
==========================================
+ Hits 97214 97680 +466
- Misses 601 608 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks Nour for the review! Could you help take a look at the Design and taking another look at the PR? Thanks! @NoureldinYosri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall looks good ... some naming comments
| busy_moment_range_by_qubit = _calc_busy_moment_range_of_each_qubit(orig_circuit) | ||
| labeled_circuit = _LabeledCircuit.from_circuit(orig_circuit, single_qubit_gate_moments_only) | ||
|
|
||
| print(f"Preprocessed input circuit repr:\n{repr}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are 3 options here
- remove this print line
- hide it behind a
if verbose:and set verbose to default to False - turn it into a logging.info call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
Found TransformerLogger in TransformerContext. Updated with TransformerLogger
Users may use use context.logger.show() to print the debugging info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @babacry
|
Thanks Nour for the prompt review!! |
Previous version of dd allows adding new moments to the circuit in the transformer. In some use cases, we might see a lot of new moments in the transformed circuits. This PR will fix the issues.
First, it captures the circuit's structure. Second, it inserts elements based on the structural information gathered in the initial step: the design.